Add TZ data to PHP config#541
Conversation
WalkthroughThe pull request adds PHP timezone configuration to the Dockerfile. A ChangesTimezone Configuration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
grocy/Dockerfile (1)
74-74: 💤 Low valueNitpick: Remove trailing space from comment.
The comment has a trailing space.
✨ Proposed fix
-# Set Timezone for PHP runtime +# Set Timezone for PHP runtime🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@grocy/Dockerfile` at line 74, Remove the trailing space at the end of the Dockerfile comment "# Set Timezone for PHP runtime " so it becomes "# Set Timezone for PHP runtime" — edit the comment line in grocy/Dockerfile to trim the trailing whitespace.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@grocy/Dockerfile`:
- Around line 74-76: Remove the broken Dockerfile RUN that uses single quotes to
write "${TZ}" (it prevents shell expansion) and instead configure PHP timezone
at container startup in the init-grocy/run oneshot s6 service; specifically,
delete the printf RUN from the Dockerfile and add startup logic in
init-grocy/run (before the directory-existence check) that logs a debug message
and writes a PHP ini file setting date.timezone using the runtime TZ environment
variable with a sensible default (e.g., UTC).
---
Nitpick comments:
In `@grocy/Dockerfile`:
- Line 74: Remove the trailing space at the end of the Dockerfile comment "# Set
Timezone for PHP runtime " so it becomes "# Set Timezone for PHP runtime" — edit
the comment line in grocy/Dockerfile to trim the trailing whitespace.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| # Set Timezone for PHP runtime | ||
| RUN printf '[PHP]\ndate.timezone = "${TZ}"\n' > /etc/php85/conf.d/50-tzone.ini | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for existing init/entrypoint scripts where runtime config should be added
fd -t f -e sh -e bash . rootfs/etcRepository: hassio-addons/app-grocy
Length of output: 165
🏁 Script executed:
#!/bin/bash
# Explore repository structure
echo "=== Repository structure ==="
git ls-files | head -30
echo ""
echo "=== Check for rootfs directory ==="
ls -la | grep -E '^d' | head -20
echo ""
echo "=== Search for init, docker-entrypoint, or startup scripts ==="
fd -t f \( -name "entrypoint*" -o -name "init*" -o -name "startup*" \)
echo ""
echo "=== Directory tree of grocy folder ==="
fd -t d . grocy | head -20Repository: hassio-addons/app-grocy
Length of output: 2338
🏁 Script executed:
#!/bin/bash
# Read the full Dockerfile to understand context
echo "=== Dockerfile around lines 74-76 ==="
cat -n grocy/Dockerfile | sed -n '65,85p'
echo ""
echo "=== Check init-grocy service directory ==="
fd -t f . grocy/rootfs/etc/s6-overlay/s6-rc.d/init-grocy
echo ""
echo "=== Check for any existing PHP config files in rootfs ==="
fd -t f . grocy/rootfs/etc/php85/
echo ""
echo "=== Search for any timezone references in the codebase ==="
rg -i "timezone|TZ" grocy/Repository: hassio-addons/app-grocy
Length of output: 1321
🏁 Script executed:
#!/bin/bash
# Check the init-grocy run script
echo "=== init-grocy/run script ==="
cat -n grocy/rootfs/etc/s6-overlay/s6-rc.d/init-grocy/run
echo ""
echo "=== Existing PHP config (99-grocy.ini) ==="
cat -n grocy/rootfs/etc/php85/conf.d/99-grocy.ini
echo ""
echo "=== Check what type of service init-grocy is ==="
cat grocy/rootfs/etc/s6-overlay/s6-rc.d/init-grocy/typeRepository: hassio-addons/app-grocy
Length of output: 1962
🏁 Script executed:
#!/bin/bash
# Check Home Assistant addon documentation patterns
echo "=== Check config.yaml for TZ environment variable ==="
cat -n grocy/config.yaml
echo ""
echo "=== Check build.yaml ==="
cat -n grocy/build.yamlRepository: hassio-addons/app-grocy
Length of output: 3372
Critical: Variable substitution won't work—feature is broken.
The single quotes in the printf command prevent shell expansion, so the literal string "${TZ}" will be written to the PHP ini file. Additionally, TZ is not defined as a build ARG, so this configuration cannot access runtime environment variables.
This addon uses s6-overlay service management. The correct approach is to configure the timezone at container startup in the init-grocy/run script (a oneshot s6-overlay service), which has access to the runtime TZ environment variable from Home Assistant:
🔧 Recommended fix
-
Remove lines 74-76 from the Dockerfile (the broken RUN command)
-
Add timezone configuration to
grocy/rootfs/etc/s6-overlay/s6-rc.d/init-grocy/runbefore the existing logic:
# After the shebang and comments, before the directory existence check:
# Configure PHP timezone from Home Assistant runtime
bashio::log.debug "Configuring PHP timezone..."
printf '[PHP]\ndate.timezone = %s\n' "${TZ:-UTC}" > /etc/php85/conf.d/50-tzone.iniThis way, the timezone is configured at container startup using the TZ environment variable provided by Home Assistant, and users can change it without rebuilding the image.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@grocy/Dockerfile` around lines 74 - 76, Remove the broken Dockerfile RUN that
uses single quotes to write "${TZ}" (it prevents shell expansion) and instead
configure PHP timezone at container startup in the init-grocy/run oneshot s6
service; specifically, delete the printf RUN from the Dockerfile and add startup
logic in init-grocy/run (before the directory-existence check) that logs a debug
message and writes a PHP ini file setting date.timezone using the runtime TZ
environment variable with a sensible default (e.g., UTC).
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions. |
Proposed Changes
Writes the TZ environment variable on the PHP config directory as is used by grocy
Related Issues
Solves the following issues:
#375
#362
#274
Summary by CodeRabbit